feat(agent): self-modify via posthog-code-internal MCP + YAML-driven tool registry by sakce · Pull Request #2415 · PostHog/code

sakce · 2026-05-28T14:44:31Z

Problem

Code can't do anything to itself. I find that annoying. We need to let it modify itself.

Changes

A localhost HTTP MCP server (posthog-code-internal) starts with the main process and dies on quit, exposing the agent's own internals as MCP tools. It mirrors McpProxyService: binds 127.0.0.1:0, generates a per-boot random bearer token, registers @preDestroy cleanup. AgentAuthAdapter appends one entry to the agent's MCP server list so every session sees it.

Tools are generated from tRPC procedures via a YAML allowlist (apps/code/src/main/services/posthog-code-internal-mcp/mcp-tools.yaml), not hand-written. At boot the registry walks appRouter, looks up each YAML-enabled procedure, converts its Zod input schema to a JSON schema, and registers it with the MCP server. Default-deny: any new tRPC procedure stays inaccessible until explicitly flipped to enabled: true. A scaffolder (pnpm --filter code scaffold-mcp-tools) keeps the YAML in sync with the router.

Initial tools enabled:

custom_instructions.read / custom_instructions.write — read/write the user's custom instructions; renderer settings store syncs via tRPC subscription when the agent writes
mcp_installations.list / mcp_installations.install — list and install MCP servers on the active project; OAuth flow falls back to returning a redirectUrl in the tool response

Adding more tools is a one-line YAML edit and (sometimes) a tRPC procedure. The four hand-written tool handlers from the first cut have been deleted; their logic lives in two new services (CustomInstructionsService, McpInstallationsService) reached through normal tRPC procedures.

How did you test this?

Manually installed the Linear MCP server (OAuth) and the NYC subway MCP (API key) end-to-end via the agent
Unit tests for CustomInstructionsService, McpInstallationsService (incl. OAuth polling), tool-registry, YAML schema validation
Typecheck, lint, full unit test suite (1563 passing) all green locally after rebase on main

Publish to changelog?

no

Generated-By: PostHog Code
Task-Id: 934a7fa1-fbb6-4f1d-89d3-f6abc69b7e23

Adds a localhost HTTP MCP server, started with the main process, that exposes the agent's own internals as MCP tools generated from tRPC procedures via a YAML allowlist. Initial tools (custom instructions read/write, MCP server list/install) are hand-picked, but adding more is just flipping enabled: true in mcp-tools.yaml. Generated-By: PostHog Code Task-Id: 934a7fa1-fbb6-4f1d-89d3-f6abc69b7e23

greptile-apps · 2026-05-28T14:54:33Z

Security Review

Timing-unsafe bearer token validation (posthog-code-internal-mcp/service.ts line 114): the per-boot token is compared with plain string equality, which is technically subject to timing side-channels. Mitigated in practice by 127.0.0.1-only binding, but crypto.timingSafeEqual is the correct primitive for credential comparisons.
No secrets are logged or leaked. The bearer token is generated with randomBytes(32) and cleared on stop().
The MCP server binds exclusively to 127.0.0.1:0; no externally reachable surface is created.
createCaller({}) with an empty context is safe for the current routers since they resolve dependencies via DI rather than tRPC context.

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
apps/code/src/main/services/posthog-code-internal-mcp/service.ts:113-117
The bearer token is validated with a plain string equality comparison, which is subject to timing side-channels. Even though the server binds only to `127.0.0.1`, a constant-time comparison is the right practice for any credential check.

```suggestion
    const auth = req.headers.authorization;
    const expected = `Bearer ${this.bearerToken}`;
    const authBuf = Buffer.from(auth ?? "");
    const expectedBuf = Buffer.from(expected);
    const validLength = authBuf.length === expectedBuf.length;
    const safeCompare = Buffer.alloc(expectedBuf.length);
    authBuf.copy(safeCompare, 0, 0, Math.min(authBuf.length, safeCompare.length));
    if (!auth || !validLength || !require("node:crypto").timingSafeEqual(safeCompare, expectedBuf)) {
      res.writeHead(401).end("Unauthorized");
      return;
    }
```

### Issue 2 of 3
apps/code/src/main/services/mcp-installations/service.ts:131-134
`pollForOauthCompletion` is fired and forgotten with no cancellation signal. If the app is quit or the service is stopped while a poll is in progress, the loop will keep making authenticated HTTP requests for up to 10 minutes with nothing to stop it. Passing an `AbortSignal` into the loop and wiring it through `@preDestroy` would prevent the leak.

```suggestion
  private async pollForOauthCompletion(
    installationId: string | undefined,
    name: string,
    signal?: AbortSignal,
  ): Promise<void> {
```

### Issue 3 of 3
apps/code/src/main/services/agent/service.test.ts:200-204
**Misleading mock name**

The last constructor argument being mocked here is `McpInstallationsService` (the injected `mcpInstallations` parameter), but it is named `internalMcp` in the test helper. The same name correctly refers to `PostHogCodeInternalMcpService` in `auth-adapter.test.ts`, so a reader cross-referencing both files will be confused about which service is actually being faked.

_{Reviews (1): Last reviewed commit: "feat(agent): add self-modification via p..." | Re-trigger Greptile}

greptile-apps · 2026-05-28T14:54:37Z

+    const auth = req.headers.authorization;
+    if (!auth || auth !== `Bearer ${this.bearerToken}`) {
+      res.writeHead(401).end("Unauthorized");
+      return;
+    }


The bearer token is validated with a plain string equality comparison, which is subject to timing side-channels. Even though the server binds only to 127.0.0.1, a constant-time comparison is the right practice for any credential check.

Suggested change

const auth = req.headers.authorization;

if (!auth || auth !== `Bearer ${this.bearerToken}`) {

res.writeHead(401).end("Unauthorized");

return;

}

const auth = req.headers.authorization;

const expected = `Bearer ${this.bearerToken}`;

const authBuf = Buffer.from(auth ?? "");

const expectedBuf = Buffer.from(expected);

const validLength = authBuf.length === expectedBuf.length;

const safeCompare = Buffer.alloc(expectedBuf.length);

authBuf.copy(safeCompare, 0, 0, Math.min(authBuf.length, safeCompare.length));

if (!auth || !validLength || !require("node:crypto").timingSafeEqual(safeCompare, expectedBuf)) {

res.writeHead(401).end("Unauthorized");

return;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/code/src/main/services/posthog-code-internal-mcp/service.ts Line: 113-117 Comment: The bearer token is validated with a plain string equality comparison, which is subject to timing side-channels. Even though the server binds only to `127.0.0.1`, a constant-time comparison is the right practice for any credential check. ```suggestion const auth = req.headers.authorization; const expected = `Bearer ${this.bearerToken}`; const authBuf = Buffer.from(auth ?? ""); const expectedBuf = Buffer.from(expected); const validLength = authBuf.length === expectedBuf.length; const safeCompare = Buffer.alloc(expectedBuf.length); authBuf.copy(safeCompare, 0, 0, Math.min(authBuf.length, safeCompare.length)); if (!auth || !validLength || !require("node:crypto").timingSafeEqual(safeCompare, expectedBuf)) { res.writeHead(401).end("Unauthorized"); return; } ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-05-28T14:54:38Z

+  private async pollForOauthCompletion(
+    installationId: string | undefined,
+    name: string,
+  ): Promise<void> {


pollForOauthCompletion is fired and forgotten with no cancellation signal. If the app is quit or the service is stopped while a poll is in progress, the loop will keep making authenticated HTTP requests for up to 10 minutes with nothing to stop it. Passing an AbortSignal into the loop and wiring it through @preDestroy would prevent the leak.

Suggested change

private async pollForOauthCompletion(

installationId: string | undefined,

name: string,

): Promise<void> {

private async pollForOauthCompletion(

installationId: string | undefined,

name: string,

signal?: AbortSignal,

): Promise<void> {

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/code/src/main/services/mcp-installations/service.ts Line: 131-134 Comment: `pollForOauthCompletion` is fired and forgotten with no cancellation signal. If the app is quit or the service is stopped while a poll is in progress, the loop will keep making authenticated HTTP requests for up to 10 minutes with nothing to stop it. Passing an `AbortSignal` into the loop and wiring it through `@preDestroy` would prevent the leak. ```suggestion private async pollForOauthCompletion( installationId: string | undefined, name: string, signal?: AbortSignal, ): Promise<void> { ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-05-28T14:54:40Z

      addAdditionalDirectory: vi.fn(),
      removeAdditionalDirectory: vi.fn(),
    },
+    internalMcp: {
+      on: vi.fn(),


Misleading mock name

The last constructor argument being mocked here is McpInstallationsService (the injected mcpInstallations parameter), but it is named internalMcp in the test helper. The same name correctly refers to PostHogCodeInternalMcpService in auth-adapter.test.ts, so a reader cross-referencing both files will be confused about which service is actually being faked.

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/code/src/main/services/agent/service.test.ts Line: 200-204 Comment: **Misleading mock name** The last constructor argument being mocked here is `McpInstallationsService` (the injected `mcpInstallations` parameter), but it is named `internalMcp` in the test helper. The same name correctly refers to `PostHogCodeInternalMcpService` in `auth-adapter.test.ts`, so a reader cross-referencing both files will be confused about which service is actually being faked. How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

…based scaffolder Greptile review on PR #2415: - Timing-safe bearer-token check via crypto.timingSafeEqual (constant-time buffer compare) instead of plain string equality. - pollForOauthCompletion now wires through an AbortController owned by McpInstallationsService and fired in @preDestroy, so in-flight polls stop when the app quits instead of running for up to 10 minutes. - Renamed the mock variable internalMcp -> mcpInstallations in agent/service.test.ts so it matches the actual injected service name. Also: rewrote scripts/scaffold-mcp-tools.ts to use the TypeScript compiler API for static AST parsing of router.ts + sub-routers, instead of dynamic import via a tsx loader. The previous approach hit ESM/CJS interop issues (node-machine-id, then named-export detection on .ts files loaded as CJS) that needed brittle per-module stubs. AST parsing has no runtime imports and runs in any Node version. The mcp-tools.yaml now lists all 256 live procedures (4 curated entries unchanged, 252 enabled: false stubs). Generated-By: PostHog Code Task-Id: 934a7fa1-fbb6-4f1d-89d3-f6abc69b7e23

greptile-apps Bot reviewed May 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(agent): self-modify via posthog-code-internal MCP + YAML-driven tool registry#2415

feat(agent): self-modify via posthog-code-internal MCP + YAML-driven tool registry#2415
sakce wants to merge 2 commits into
mainfrom
feat/code-self-modify

sakce commented May 28, 2026

Uh oh!

greptile-apps Bot commented May 28, 2026

Security Review

Uh oh!

greptile-apps Bot May 28, 2026

Uh oh!

greptile-apps Bot May 28, 2026

Uh oh!

greptile-apps Bot May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sakce commented May 28, 2026

Problem

Changes

How did you test this?

Publish to changelog?

Uh oh!

greptile-apps Bot commented May 28, 2026

Security Review

Uh oh!

greptile-apps Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant